feat(governance): add ruleset tools with dynamic scope challenges - #2991

pkg/github/rulesets.go:780

• This validation only checks that each rule type survived. For a recognized type, unknown/misspelled parameters fields are silently dropped by the typed JSON round-trip; unknown conditions fields are likewise ignored. The request can therefore create a materially weaker or broader ruleset than requested. Validate the complete round-tripped payload (including nested fields, ordering/multiplicity, conditions, and bypass actors), or expose strict schemas for these structures.

	// github.RepositoryRulesetRules.UnmarshalJSON silently discards rule types it
	// does not recognize, which would let a typo create a weaker ruleset than the
	// caller requested. Verify every requested rule type survived the round-trip.

pkg/github/rulesets.go:363

• Install the response-body close before checking err; go-github can return a response with an error, and the current ordering leaks that body/connection.

	rulesets, resp, err := client.Repositories.GetAllRulesets(ctx, owner, repo, opts)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rulesets", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:379

• Install the response-body close before checking err; otherwise failed branch-rule requests can leave the returned response body open.

	branchRules, resp, err := client.Repositories.ListRulesForBranch(ctx, owner, repo, branch, opts)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rules for branch", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:455

• client.Do can return both resp and err; because the defer is below the error return, failed rule-suite requests leak the response body. Close any non-nil response before the error check.

	resp, err := client.Do(req, &ruleSuites)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rule suites", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:475

• client.Do can return both resp and err; install the close before the error return so failed rule-suite lookups do not leak the body/connection.

	resp, err := client.Do(req, &ruleSuite)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rule suite", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:487

• Defer closing a non-nil response before checking err; organization API errors may still return a response body that otherwise remains open.

	ruleset, resp, err := client.Organizations.GetRepositoryRuleset(ctx, org, rulesetID)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get organization repository ruleset", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:504

• Defer closing a non-nil response before checking err; failed organization-list calls can return an open response body.

	rulesets, resp, err := client.Organizations.ListAllRepositoryRulesets(ctx, org, opts)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list organization repository rulesets", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:516

• Defer closing a non-nil response before checking err; enterprise API errors may include a response body that must be closed.

	ruleset, resp, err := client.Enterprise.GetRepositoryRuleset(ctx, enterprise, rulesetID)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get enterprise repository ruleset", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:547

• Move response cleanup ahead of the error return. client.Do can return a non-nil response on an API error, so this ordering leaks failed enterprise-list responses.

	resp, err := client.Do(req, &rulesets)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list enterprise repository rulesets", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:613

• Defer closing a non-nil response before the error check; failed repository create calls can return an open response body.

				created, resp, err := client.Repositories.CreateRuleset(ctx, owner, repo, ruleset)
				if err != nil {
					return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create repository ruleset", resp, err), nil, nil
				}
				defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:624

• Defer closing a non-nil response before the error check; otherwise organization create failures can leak their response body/connection.

				created, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, org, ruleset)
				if err != nil {
					return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create organization repository ruleset", resp, err), nil, nil
				}
				defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:635

• Defer closing a non-nil response before the error check; otherwise enterprise create failures can leak their response body/connection.

				created, resp, err := client.Enterprise.CreateRepositoryRuleset(ctx, enterprise, ruleset)
				if err != nil {
					return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create enterprise repository ruleset", resp, err), nil, nil
				}
				defer func() { _ = resp.Body.Close() }()

Previously missed (1) — in code that hasn't changed since the last review.

pkg/github/rulesets.go:70

• This makes the write tool visible to classic PATs even when they have none of the scopes needed by any level. Unlike the read tool, creation has no public unauthenticated path; use ANY-of visibility for repo, admin:org, or admin:enterprise so unsupported tools are filtered out, and update the visibility assertion accordingly.

		func([]string) bool { return true },

pkg/github/rulesets.go:47

• The callback is case-sensitive, but the handler later dispatches with strings.ToLower(level). A call with level: "Repository" therefore skips the OAuth challenge and still reaches the repository API. Normalize here as well (or reject mixed case in the handler) so accepted calls cannot bypass up-scoping.

			switch level {

pkg/github/rulesets.go:76

• The handler accepts mixed-case levels via strings.ToLower(level), while this pre-handler scope callback does not. For example, level: "Enterprise" reaches enterprise creation without producing the required admin:enterprise challenge. Apply the same normalization in both paths.

			switch level {

pkg/github/rulesets.go:343

• Close the response body before checking err. go-github can return a non-nil response with an open body on API errors, so the current early return leaks the connection instead of making it reusable.

	ruleset, resp, err := client.Repositories.GetRuleset(ctx, owner, repo, rulesetID, includesParents)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository ruleset", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:363

• This returns before closing the response body on GitHub API errors. Defer closure immediately after the client call so failed list requests do not leak transport connections.

	rulesets, resp, err := client.Repositories.GetAllRulesets(ctx, owner, repo, opts)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rulesets", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:379

• An error response may still contain an open body, but this path returns before the defer is installed. Close non-nil responses before checking err to avoid leaking connections.

	branchRules, resp, err := client.Repositories.ListRulesForBranch(ctx, owner, repo, branch, opts)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rules for branch", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:455

• client.Do can return both an error and a response whose body must be closed. Install the guarded defer before the error return so repeated failed rule-suite calls do not exhaust idle connections.

	resp, err := client.Do(req, &ruleSuites)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list repository rule suites", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:475

• The error path returns before closing a non-nil response body from client.Do. Move a nil-guarded defer ahead of the error check to preserve HTTP connection reuse.

	resp, err := client.Do(req, &ruleSuite)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get repository rule suite", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:487

• A failed organization ruleset lookup can still return a response with an open body. Guard and defer the close before checking err so the error path does not leak the connection.

	ruleset, resp, err := client.Organizations.GetRepositoryRuleset(ctx, org, rulesetID)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get organization repository ruleset", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:504

• The response body is only closed on success. Since go-github returns responses for HTTP errors too, defer a guarded close before the early return.

	rulesets, resp, err := client.Organizations.ListAllRepositoryRulesets(ctx, org, opts)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list organization repository rulesets", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:516

• This early error return leaves the response body open when GitHub returns an HTTP error. Install the nil-guarded defer before checking err.

	ruleset, resp, err := client.Enterprise.GetRepositoryRuleset(ctx, enterprise, rulesetID)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get enterprise repository ruleset", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:547

• client.Do may return a non-nil response on failure, but the current return bypasses body closure. Move a guarded defer before the error check to avoid connection leaks.

	resp, err := client.Do(req, &rulesets)
	if err != nil {
		return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to list enterprise repository rulesets", resp, err), nil
	}
	defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:613

• Repository creation returns before closing response bodies attached to GitHub API errors. Defer a guarded close immediately after the call so failed create attempts do not leak transport resources.

				created, resp, err := client.Repositories.CreateRuleset(ctx, owner, repo, ruleset)
				if err != nil {
					return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create repository ruleset", resp, err), nil, nil
				}
				defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:624

• The organization create error path skips response-body closure. A non-nil error response must be closed before returning to keep the HTTP transport reusable.

				created, resp, err := client.Organizations.CreateRepositoryRuleset(ctx, org, ruleset)
				if err != nil {
					return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create organization repository ruleset", resp, err), nil, nil
				}
				defer func() { _ = resp.Body.Close() }()

pkg/github/rulesets.go:635

• The enterprise create path leaks response bodies on API errors because the defer is installed only after the error check. Close any non-nil response before returning.

				created, resp, err := client.Enterprise.CreateRepositoryRuleset(ctx, enterprise, ruleset)
				if err != nil {
					return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to create enterprise repository ruleset", resp, err), nil, nil
				}
				defer func() { _ = resp.Body.Close() }()

Previously missed (4) — in code that hasn't changed since the last review.

pkg/github/rulesets.go:53

• The organization and enterprise GET ruleset endpoints require admin:org and admin:enterprise, respectively, for OAuth apps and classic PATs; read:org/read:enterprise are insufficient. These challenges can therefore complete successfully with a token that the API immediately rejects with 403. Challenge for the admin scopes here and update the exhaustive scope list, tests, snapshots, and generated docs accordingly.

			case "organization":
				return scopes.ChallengeAll(activeScopes, scopes.ReadOrg)
			case "enterprise":
				return scopes.ChallengeAll(activeScopes, scopes.ReadEnterprise)

pkg/github/rulesets.go:396

• The rule-suite endpoint also supports the documented evaluate_status filter (all, active, or evaluate), but this filter model omits it while exposing every other endpoint filter. Callers consequently cannot restrict results to evaluation-mode or active rulesets. Add it to the input schema, argument parsing, query construction, and regression coverage.

// ruleSuiteFilters holds the optional filters for listing rule suites.
type ruleSuiteFilters struct {
	Ref             string
	TimePeriod      string
	ActorName       string
	RuleSuiteResult string
}

pkg/github/rulesets.go:456

• Decoding an untyped response into any converts every JSON number to float64. Rule-suite IDs and actor IDs are 64-bit values, so values above 2^53 are rounded when MarshalledTextResult encodes them again. Decode into json.RawMessage to preserve the API response exactly.

This issue also appears on line 478 of the same file.

	var ruleSuites any

pkg/github/rulesets.go:558

• Enterprise ruleset objects contain 64-bit numeric IDs, but decoding into any converts them to float64 and can change their values when the result is re-marshaled. Use json.RawMessage so the direct API response retains integer precision.

	var rulesets any

pkg/github/rulesets.go:478

• This untyped decode converts 64-bit IDs in the rule-suite response to float64, which silently rounds values above 2^53 before returning them to the caller. Preserve the raw JSON instead.

	var ruleSuite any

Previously missed (1) — in code that hasn't changed since the last review.

pkg/github/rulesets.go:70

• The write tool is visible to every classic PAT, including tokens with none of the scopes that can authorize any of its operations. This defeats the startup filtering documented in docs/scope-filtering.md:9-11 and leaves users with an unusable write tool. Make visibility true only when the PAT has at least one of repo, admin:org, or admin:enterprise, and update the shared visibility assertion in the scope tests accordingly.

		func([]string) bool { return true },

Previously missed (2) — in code that hasn't changed since the last review.

pkg/github/rulesets.go:379

• This forwards the branch name as a raw path segment. Valid branch names commonly contain / (for example feature/login), so go-github builds /rules/branches/feature/login instead of encoding the branch as feature%2Flogin, and the endpoint does not match the requested branch. Escape the branch before passing it to this go-github method and add a slash-containing regression case.

	branchRules, resp, err := client.Repositories.ListRulesForBranch(ctx, owner, repo, branch, opts)

pkg/github/rulesets.go:738

• Unknown top-level arguments are silently ignored when this map is converted into the outbound payload. Since this repository registers the raw mcp.AddTool handler and only unmarshals arguments into a map, the input schema does not reject additional properties; for example, condition instead of conditions creates the ruleset without any applicability condition. Reject unrecognized keys before constructing this governance request.

func buildRepositoryRulesetFromArgs(args map[string]any) (github.RepositoryRuleset, *mcp.CallToolResult) {

pkg/github/rulesets.go:764

• Only type and parameters survive go-github's ruleset-rule unmarshal, but extra keys on each rule object are not checked. A common typo such as parameter (singular) is therefore discarded and a pull_request rule is sent with zero/default parameters, potentially creating a weaker rule than requested. Reject every rule-object key other than type and parameters, as is already done for bypass actors.

	for _, rule := range rules {
		ruleMap, ok := rule.(map[string]any)
		if !ok {
			return github.RepositoryRuleset{}, utils.NewToolResultError("each rule must be an object with a 'type' field")
		}